Skip to content

fix: revert np.where to xarray.where when adding vars/ constraints#575

Merged
FabianHofmann merged 3 commits intoPyPSA:masterfrom
lkstrp:fix
Feb 10, 2026
Merged

fix: revert np.where to xarray.where when adding vars/ constraints#575
FabianHofmann merged 3 commits intoPyPSA:masterfrom
lkstrp:fix

Conversation

@lkstrp
Copy link
Member

@lkstrp lkstrp commented Feb 9, 2026

#555 introduced masking and used np.where instead of xarray.where since is is "38x faster than xarray where".

But xarray is slower for a reason, since it aligns operands by dimension name, not just position. np.where requires shapes to match positionally, which is not guaranteed in add_variables or add_constraints. This PR just reverts the changes, we should add tests and release a patch tomorrow.

See https://github.com/PyPSA/linopy/pull/555/changes#diff-1a9bf239769f2af33c90e00a21f7f08e2f40d626ce06ee470bb2daa65ac06a04R788-R789

@FBumann
Copy link
Contributor

FBumann commented Feb 9, 2026

@lkstrp Sorry if this causes troubles.

@FabianHofmann
Copy link
Collaborator

FabianHofmann commented Feb 10, 2026

@lkstrp Sorry if this causes troubles.

no this is rather on me, I though we had the internal constraint already that masks have to have the same shape as constraints/variables

FabianHofmann
FabianHofmann approved these changes Feb 10, 2026
View reviewed changes
Copy link
Collaborator

@FabianHofmann FabianHofmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for spotting this so quickly @lkstrp

@lkstrp
Copy link
Member Author

lkstrp commented Feb 10, 2026

no this is rather on me, I though we had the internal constraint already that masks have to have the same shape as constraints/variables

We could also make this explicit with a test and error message, and break things for a couple of people. Or revert and break with next minor release

@lkstrp Sorry if this causes troubles

No worries, those things can slip through. It just gets more likely with the vast amount of code produced these days, and we need to watch out

@FBumann
Copy link
Contributor

FBumann commented Feb 10, 2026

Yes, PR's are getting easier those days, as you can see from mine i guess...
Im aware of the switch in effort for PR creation vs reviewing, so tell me if i can improve your review experience.

@lkstrp lkstrp mentioned this pull request Feb 10, 2026
Merged
@FabianHofmann
Copy link
Collaborator

FabianHofmann commented Feb 10, 2026 •
edited
Loading

sorry, for the gibberish in the last comment, I was in a meeting. I would say let's revert this, in particular given the failing pypsa functions. then updating linopy to the latest and keeping pypsa v1.0.x does not break. But I would like to require mask to be properly aligned in the first place. Linopy already has so many reindexing logics that you sometimes don't know what's going on...

@FabianHofmann FabianHofmann merged commit 606a714 into PyPSA:master Feb 10, 2026
20 checks passed
lkstrp added a commit that referenced this pull request Feb 10, 2026
@lkstrp
test and future warning for #575 (#579)
ec6262b 
* trigger

* test: add test

* add future warning

* fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@FabianHofmann FabianHofmann FabianHofmann approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@lkstrp @FBumann @FabianHofmann